Graduate resource sharing feature out of experimental - #6348
DarshitChanpura merged 2 commits into
Conversation
PR Reviewer Guide 🔍(Review updated until commit dbcb3c0)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to dbcb3c0 Explore these optional code suggestions:
Previous suggestionsSuggestions up to commit 4954a66
Suggestions up to commit 739106b
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6348 +/- ##
==========================================
+ Coverage 75.97% 75.98% +0.01%
==========================================
Files 461 461
Lines 30920 30920
Branches 4668 4668
==========================================
+ Hits 23492 23496 +4
+ Misses 5270 5268 -2
+ Partials 2158 2156 -2
🚀 New features to boost your workflow:
|
Renames the resource sharing feature-flag settings, dropping the
'.experimental' segment (breaking change, no deprecated fallback):
plugins.security.experimental.resource_sharing.enabled
-> plugins.security.resource_sharing.enabled
plugins.security.experimental.resource_sharing.protected_types
-> plugins.security.resource_sharing.protected_types
Also removes the @opensearch.experimental annotations from the
resource-sharing SPI and implementation classes, and updates
RESOURCE_SHARING_AND_ACCESS_CONTROL.md accordingly.
Related to opensearch-project#4500
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
739106b to
4954a66
Compare
|
Persistent review updated to latest commit 4954a66 |
|
@DarshitChanpura until there's a clear path with workspaces like proposed in #6119 I don't think this should be promoted out of experimental. Let's get a consistent platform level experience for sharing. I'm very strongly supportive of the ownership model where owner gets to choose sharing and I think the fact that workspaces was developed in a way where workspaces have owners that invite collaborators is testament to that being the desired model. Workspace collaborators being controlled by the frontend is a problem and we need to figure out the path forward that works with workspaces desire to be supported with MDS (Multiple Data Sources). If that problem can be solved then there's clear line-of-sight to graduating from experimental. Edit: @DarshitChanpura can we focus the efforts on adding a share button inside a dashboards plugin (like reporting) that at least with multi-tenancy is tenant aware. idk if reporting plugin is workspace aware atm and I know some teams had this notion of workspace acl (how does that work?) If we can show MDS working with resource sharing where the currently toggled cluster supports resource sharing and share button appears than that would probably be a good place to begin. We certainly are going to want to work backwards from UX so that these share APIs can be conveniently called from the frontend and we should also have something similar to #6107 to power new experiences on Dashboards where we can enable/disable actions based on user permissions. |
Hahaa, here is the centralized share button PR: opensearch-project/security-dashboards-plugin#2491. I've used AD as example but you get the gist.
While I agree that workspaces could leverage from the ownership model, I disagree on not taking this feature out of experimental. We should not delay this feature by tying it with workspaces improvement. |
No they need to be compatible and I would block on that. Let's figure out the path forward. |
|
Persistent review updated to latest commit dbcb3c0 |
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types - Renames the setting keys across the 12 pages that document them. - Removes the experimental warning callout from the 13 resource sharing pages, including the resource sharing API reference. - Drops the pointers to the experimental feature flags page, which no longer applies to these settings. - Adds an upgrade note on the security resource sharing page: the old keys are not recognized, a node that still has one fails to start, and a persistent cluster setting under an old key is archived on upgrade. The "Introduced 3.3" labels and the 3.3/3.4 static-versus-dynamic notes are left as is; they still describe the feature's history accurately. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
|
Related workspaces PR has been merged and this one is unblocked now |
c59f017
into
opensearch-project:main
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings and the resource-sharing toggle helper in PluginRestTestCase to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types - Renames the setting keys across the 12 pages that document them. - Removes the experimental warning callout from the 13 resource sharing pages, including the resource sharing API reference. - Drops the pointers to the experimental feature flags page, which no longer applies to these settings. - Adds an upgrade note on the security resource sharing page: the old keys are not recognized, a node that still has one fails to start, and a persistent cluster setting under an old key is archived on upgrade. The "Introduced 3.3" labels and the 3.3/3.4 static-versus-dynamic notes are left as is; they still describe the feature's history accurately. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types - Renames the setting keys across the 12 pages that document them. - Removes the experimental warning callout from the 13 resource sharing pages, including the resource sharing API reference. - Drops the pointers to the experimental feature flags page, which no longer applies to these settings. - Adds an upgrade note on the security resource sharing page: the old keys are not recognized, a node that still has one fails to start, and a persistent cluster setting under an old key is archived on upgrade. The "Introduced 3.3" labels and the 3.3/3.4 static-versus-dynamic notes are left as is; they still describe the feature's history accurately. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings and the cluster-setting toggles and javadoc in RscMigrateE2ERestApiIT to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
…cated Graduating resource sharing out of experimental (opensearch-project#6348) renamed the two feature-flag settings and dropped the old names outright: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types An existing cluster therefore had to be reconfigured before upgrading. A node whose opensearch.yml still carried an old key would not start, and a persistent cluster setting under an old key was archived on upgrade, silently returning the feature to its default of disabled. Consumers then fall back to their own access control without reporting an error, so shares stop being honored with no signal. This restores the old names as deprecated aliases instead, matching how the plugin already handles the opendistro-to-opensearch rename of ssl_dual_mode_enabled: - Each current setting now declares its pre-graduation setting as a fallback. The fallback resolves at read time against whichever settings instance is supplied, so it covers opensearch.yml, cluster settings and dynamic updates alike. The current name takes precedence when both are set, so a leftover old key cannot override a deliberate new one. - The old settings are registered and marked Property.Deprecated, which is also what lets the upgraders below resolve them. - A setting upgrader per setting rewrites an old key to the current one during cluster-state recovery and on any cluster settings update that still uses it, so an upgraded cluster stops carrying the deprecated key rather than keeping it indefinitely. - Because the deprecation warning core emits does not name a replacement by design, each setting logs an explicit warning naming the current key when the old one is in use. Users keep working through an upgrade, are told the old names are going away, and are pointed at the replacement. Testing: new ResourceSharingSettingMigrationTests covers fallback resolution for both settings, precedence when both keys are set, the upgraders' key mapping, and upgradeSettings rewriting the old keys while leaving current ones untouched. Verified green, then confirmed the fallback tests fail when the fallback wiring is removed. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Resource sharing graduates out of experimental in opensearch-project/security#6348, which drops the experimental. segment from the feature-flag settings with no deprecated fallback: plugins.security.experimental.resource_sharing.enabled -> plugins.security.resource_sharing.enabled plugins.security.experimental.resource_sharing.protected_types -> plugins.security.resource_sharing.protected_types Updates the integTest cluster settings to the new keys. Only the setting strings change. The ConfigConstants constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so no Java/Kotlin references are affected. The gradle -Dresource_sharing.enabled system property that gates the CI matrix is a separate toggle and is left as is. Verified no occurrence of the old key remains anywhere in the repo. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Description
Graduates the Resource Sharing & Access Control feature out of experimental.
Breaking change (no deprecated fallback): the feature-flag settings are renamed to drop the
.experimentalsegment:plugins.security.experimental.resource_sharing.enabledplugins.security.resource_sharing.enabledplugins.security.experimental.resource_sharing.protected_typesplugins.security.resource_sharing.protected_typesOnly the string values in
ConfigConstantschange — the constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED,OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so downstream Java references are unaffected.Also:
@opensearch.experimentaljavadoc annotations from the resource-sharing SPI and implementation classes (leaves the unrelatedSecurityConfigExtensionmarker in place).RESOURCE_SHARING_AND_ACCESS_CONTROL.md(setting keys + drops the "experimental" note).Because the rename is breaking and has no fallback, this must ship in 3.9 together with the downstream references. Coordinated changes are being made to:
documentation-website(setting keys + removal of experimental warning callouts)reporting,anomaly-detection,ml-commons,notifications,alerting,flow-framework,security-analyticsDraft while those companion PRs are prepared and reviewed.
Related Issues
Related to #4500
Check List
--signoffBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.